Skip to content

Conversation

@1rneh
Copy link
Contributor

@1rneh 1rneh commented Jan 8, 2026

  • Refactor updating remote policies
  • Fix merging policies of a combined provider when remote policies are updated

case "policies-startup":
// Before the first set of policy callbacks runs, we must
// initialize the service.
this._initialize();
await this._initialize();
Copy link
Contributor

@gcp gcp Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't the callers need to know about this? They would assume things are already initialized when this returns, but it's not? So you'd get new race conditions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! As temporary work around we are now spinning the event loop until the remote policies are fetched.

);

if (!isValid) {
continue;
Copy link
Contributor

@gcp gcp Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause the policy to be removed because it doesn't go in parsedPolicies, unclear if that is what you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I didn't change that behavior. But let's re-think this :)

We have two options when encountering invalid parameters:

  • Policy is currently active: We can remove the policy or keep it active with unchanged parameters. I think I prefer the latter. It's seems more intentional. In both ways we can warn in the logs about invalid parameters.
  • Policy is currently inactive: Do nothing.

@1rneh 1rneh requested a review from Mossop January 22, 2026 15:12
@1rneh 1rneh requested a review from gcp January 22, 2026 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants